Skip to content

Conversation

AlfieRichardsArm
Copy link
Contributor

This changes the type of PredicationCode and VPTPredicationCode from unsigned to ARMCC::CondCodes and ARMVCC::VPTCodes resp' for clarity and correctness.

@llvmbot
Copy link
Member

llvmbot commented Feb 29, 2024

@llvm/pr-subscribers-mc

@llvm/pr-subscribers-backend-arm

Author: Alfie Richards (AlfieRichardsArm)

Changes

This changes the type of PredicationCode and VPTPredicationCode from unsigned to ARMCC::CondCodes and ARMVCC::VPTCodes resp' for clarity and correctness.


Full diff: https://github.com/llvm/llvm-project/pull/83413.diff

1 Files Affected:

  • (modified) llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp (+9-9)
diff --git a/llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp b/llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
index 37bfb76a494dee..dd6c637a6fa2a0 100644
--- a/llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
+++ b/llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
@@ -506,9 +506,10 @@ class ARMAsmParser : public MCTargetAsmParser {
 
   bool isMnemonicVPTPredicable(StringRef Mnemonic, StringRef ExtraToken);
   StringRef splitMnemonic(StringRef Mnemonic, StringRef ExtraToken,
-                          unsigned &PredicationCode,
-                          unsigned &VPTPredicationCode, bool &CarrySetting,
-                          unsigned &ProcessorIMod, StringRef &ITMask);
+                          ARMCC::CondCodes &PredicationCode,
+                          ARMVCC::VPTCodes &VPTPredicationCode,
+                          bool &CarrySetting, unsigned &ProcessorIMod,
+                          StringRef &ITMask);
   void getMnemonicAcceptInfo(StringRef Mnemonic, StringRef ExtraToken,
                              StringRef FullInst, bool &CanAcceptCarrySet,
                              bool &CanAcceptPredicationCode,
@@ -6283,10 +6284,9 @@ bool ARMAsmParser::parsePrefix(ARMMCExpr::VariantKind &RefKind) {
 //
 // FIXME: Would be nice to autogen this.
 // FIXME: This is a bit of a maze of special cases.
-StringRef ARMAsmParser::splitMnemonic(StringRef Mnemonic,
-                                      StringRef ExtraToken,
-                                      unsigned &PredicationCode,
-                                      unsigned &VPTPredicationCode,
+StringRef ARMAsmParser::splitMnemonic(StringRef Mnemonic, StringRef ExtraToken,
+                                      ARMCC::CondCodes &PredicationCode,
+                                      ARMVCC::VPTCodes &VPTPredicationCode,
                                       bool &CarrySetting,
                                       unsigned &ProcessorIMod,
                                       StringRef &ITMask) {
@@ -6340,7 +6340,7 @@ StringRef ARMAsmParser::splitMnemonic(StringRef Mnemonic,
     unsigned CC = ARMCondCodeFromString(Mnemonic.substr(Mnemonic.size()-2));
     if (CC != ~0U) {
       Mnemonic = Mnemonic.slice(0, Mnemonic.size() - 2);
-      PredicationCode = CC;
+      PredicationCode = (ARMCC::CondCodes)CC;
     }
   }
 
@@ -6387,7 +6387,7 @@ StringRef ARMAsmParser::splitMnemonic(StringRef Mnemonic,
     unsigned CC = ARMVectorCondCodeFromString(Mnemonic.substr(Mnemonic.size()-1));
     if (CC != ~0U) {
       Mnemonic = Mnemonic.slice(0, Mnemonic.size()-1);
-      VPTPredicationCode = CC;
+      VPTPredicationCode = (ARMVCC::VPTCodes)CC;
     }
     return Mnemonic;
   }

This changes the type from `unsigned` to `ARMCC::CondCodes` for clarity and
correctness.
@AlfieRichardsArm
Copy link
Contributor Author

Note the failing test is not relevant, seems to be a timeout or similar

@AlfieRichardsArm
Copy link
Contributor Author

I will wait a couple days for external code review

@AlfieRichardsArm
Copy link
Contributor Author

@s-barannikov I've added you as a reviewer as this commit is a prerequisite for some optional operands work that I will submit in coming days and I see you have worked on the same area.

Copy link
Contributor

@s-barannikov s-barannikov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Straightforward change, LGTM modulo style nit

@@ -6340,7 +6340,7 @@ StringRef ARMAsmParser::splitMnemonic(StringRef Mnemonic,
unsigned CC = ARMCondCodeFromString(Mnemonic.substr(Mnemonic.size()-2));
if (CC != ~0U) {
Mnemonic = Mnemonic.slice(0, Mnemonic.size() - 2);
PredicationCode = CC;
PredicationCode = (ARMCC::CondCodes)CC;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The anchor link really changes the meaning of that link (:
I'll get a quick fix in

Copy link

github-actions bot commented Feb 29, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@AlfieRichardsArm
Copy link
Contributor Author

Failing test continues to be a seemingly unrelated failure regarding MLIR

@AlfieRichardsArm AlfieRichardsArm force-pushed the change-cc-and-vcc-type branch from 0bc0eda to a1cd51c Compare March 1, 2024 12:44
@llvmbot llvmbot added the llvm:mc Machine (object) code label Mar 1, 2024
@AlfieRichardsArm AlfieRichardsArm force-pushed the change-cc-and-vcc-type branch from a1cd51c to bf7bbb1 Compare March 1, 2024 12:46
@AlfieRichardsArm
Copy link
Contributor Author

After a fixing me pushing to the wrong branch this is back to what it should be.

@AlfieRichardsArm
Copy link
Contributor Author

Test failure is unrelated MLIR test. Merging this now.

@AlfieRichardsArm AlfieRichardsArm merged commit b8e0f3e into llvm:main Mar 1, 2024
@AlfieRichardsArm AlfieRichardsArm deleted the change-cc-and-vcc-type branch March 1, 2024 13:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:ARM llvm:mc Machine (object) code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants